Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: basic navigation #92

Closed
wants to merge 9 commits into from
Closed

Conversation

KarlHeitmann
Copy link
Contributor

Hi!

Here is an idea I've built some weeks ago. I wanted to add navigation through the nodes of the JSON. Once upon the user clicks on a node of the graph, the id of the node is sent through props.id to the handleClick function, and the handle click will search that id on the original graph structure of the JSON. When it finds the match, the function that is applied to the original JSON main tree is applied on the subtree whose root is theid that matches props.id

The navigation feature is toggled by a new button on the sidebar. If the button is pressed by second time (thus, disabling navigation) it will reset the graph and display again the main tree.

The navigation button was originally meant to be a patch, because I wanted to add a parent node to the subtree, when this parent node is clicked, it will search for the id of the parent node of the subtree on the main tree, and then display that broader subtree on the display window.

Adding this parent node to the subtree was not a task as easy as displaying the subtree, that's the reason why I decided to add more tests to the project. You can see them on this branch on my fork

I was working on it, but when I visited the changes on the upstream repo (yours) a couple days ago, I found out that you changed the structure of Graph/index.tsx component, and you now use React.memo. I've tried doing a rebase on the upstream but I was not able to add navigation to subtrees using the new Graph/index.tsx component that uses the React.memo function. That's the reason why I am submitting this PR. Can you help me doing the merge, so I can continue working on this feature? My attempt to do the rebase is here if you want to take a look at it.

I will be on the lookout for any comment, suggestion and/or feedback over this feature.

Best regards

@@ -89,11 +89,16 @@ const relationships = (xs: { id: string; children: never[] }[]) => {
]);
};

export const flattenTree = tree => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've needed to break in two the function parser on line 97.

  • extractTree will take as input the JSON data, and outputs the node tree that contains the id and children keys. This is the main data structure to work on.
  • flattenTree is the function that will take as an input the extracted tree produced by extractTree, and will flatten it to an array. That array can be later on used to split the nodes and edges. I am missing some tests here, because on the test files you can take a look at the input/output data structures and have a better idea of what each function does, without need to read the source code of them. Take a look at this branch's folder to see the tests

@AykutSarac
Copy link
Owner

Thank you for creating such feature, I have merged the PR branch with upstream so you can continue. Good luck!

@KarlHeitmann
Copy link
Contributor Author

Thanks for the update on the branch @AykutSarac . It was very helpful, and I could make some updates to the branch.

  • I've modified the generateChildren function and added a new .map stage. It takes the array of two elements and adds a third element to the array that is a call to nextId(). I've had to add the current id on that stage to memoize it there because if I stored the current_id inside a variable in the original flatMap function, the nextId() arrow function had a strange behaviour. Maybe the two .map calls can be merged into one, I didn't exhaust all the posibilities though.

  • The modification to the extractTree were made in tandem with the modifications to generateChildren, because these two functions calls each other recursively. These modifications will add the parent_id information to each node of the tree.

  • I've modified the flattenTree function to add the parent node and the edge that goes from the root node of the subtree to the parent node . With this two more elements inside the array of flatten nodes and edges, getEdgeNodes will do the work of separating the edges and nodes so they can be sent to the canvas and draw everything. I think the modification here surely can be refactorized to look better.

Pending work:

  • I'd like to draw the parent node in a different color and at the left side of the root node of the subtree while navigating. To easily distinguish it from the real nodes of the JSON. But I don't know how to do it yet, I'd have to study the other parts of the source code (specially files src/components/CustomNode/ObjectNode.tsx and src/components/CustomNode/styles.tsx) and the reaflow library.

  • There are some linting errors.

  • I don't know if the toggle navigation button is needed anymore. If you want to erase it, we should think on a way to decide whether display the details modal or navigate to the subgraph when clicking a node on the graph.

If you need me to improve something more on this PR just say it here. I did enjoy working on your project. I have some ideas that may be interesting to build, for example:

  • Add a panel to the right called "filter". On this panel, one can add another JSON which will contain a set of rules that can be merged with the data json. There rules can be used to filter out some keys on each node of the original JSON, in order to hide a set of keys in each node.

If you need something else just ping me.

@AykutSarac
Copy link
Owner

  • I'd like to draw the parent node in a different color and at the left side of the root node of the subtree while navigating. To easily distinguish it from the real nodes of the JSON. But I don't know how to do it yet, I'd have to study the other parts of the source code (specially files src/components/CustomNode/ObjectNode.tsx and src/components/CustomNode/styles.tsx) and the reaflow library.

For this part, I think it would be better to just leave the parent node as is for now. The less customization the better it is.

  • I don't know if the toggle navigation button is needed anymore. If you want to erase it, we should think on a way to decide whether display the details modal or navigate to the subgraph when clicking a node on the graph.

I think we could put a navigation mode once this PR comes to a final, user can choose between navigation modes at sidebar. I have good UI & UX plans for this one.

If you need something else just ping me.

@KarlHeitmann Thank you so much for the contribution, it is especially so awesome when people adding new feature to the project.
The top prioritized issue with the project is #51 & #27. Perhaps you could take a look at them if you are looking for something additional :)

@KarlHeitmann
Copy link
Contributor Author

Thanks! I'm going to take a look at #51 and #27 I was looking for something additional to do.

For this part, I think it would be better to just leave the parent node as is for now. The less customization the better it is.

Ok! I like keeping things simple. However, yesterday I've had an idea that needs only two lines of code to be changed. Take a look at this branch I leaved on my fork https://github.com/KarlHeitmann/jsonvisio.com/tree/aesthetic_suggestion_to_nav

image

With these changes the graph looks something like this

image

const subTree = searchSubTree(mainTree, props.id);

if (subTree.length) {
const flatTree = flattenTree(subTree);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flattenTree from json-editor-parser.ts is applied to the sub tree whose node is clicked by the user. Here you apply the same function applied on line 64 of this file into a subtree. FRACTAL.

@@ -46,6 +44,24 @@ export function getEdgeNodes(
};
}


export function searchSubTree (trees: any[], id: string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extremely simple function to search a match id, and take a subtree from the JSON data.

@@ -0,0 +1,190 @@
import { extractTree } from "src/utils/json-editor-parser"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test files

@AykutSarac AykutSarac closed this Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants